Add Java ACA and AppService templates#799
Conversation
wbreza
left a comment
wbreza
left a comment
There was a problem hiding this comment.
Looks great overall. I would recommend someone with more Java experience review the Java specific bits.
|
@jdubois, @brunoborges, @roryp or @bbenz if any one of you can help sanity check the Java portion of changes to make sure nothing is amiss, that'd be appreciated. I'm still waiting to get a |
|
@weikanglim did you copy/paste the Java application I coded about 6 months ago? If that's the case, it should be all good. Maybe we can update it to the latest Spring Boot 2.x release, that shouldn't change anything (and remove a few CVE advisories). |
|
@jdubois That is correct. All the Java code was lifted from your existing implementation. Only slight modifications based on app properties, keyvault and appinsights integration. Upgraded spring-starter dependency as you suggested. |
|
@weikanglim then that should be all good. I'll have a look to update it to the latest Spring Boot version, but that's not urgent, it's already very recent. |
Repoman Generation ResultsRepoman pushed changes to remotes for the following projects: Project: todo-csharp-cosmos-sqlRemote: azure-samples-stagingBranch: pr/799You can initialize this project with: azd init -t Azure-Samples/todo-csharp-cosmos-sql -b pr/799View Changes | Compare Changes Project: todo-csharp-sqlRemote: azure-samples-stagingBranch: pr/799You can initialize this project with: azd init -t Azure-Samples/todo-csharp-sql -b pr/799View Changes | Compare Changes Project: todo-java-mongo-acaRemote: azure-samples-stagingBranch: pr/799You can initialize this project with: azd init -t Azure-Samples/todo-java-mongo-aca -b pr/799View Changes | Compare Changes Project: todo-java-mongoRemote: azure-samples-stagingBranch: pr/799You can initialize this project with: azd init -t Azure-Samples/todo-java-mongo -b pr/799View Changes | Compare Changes Project: todo-nodejs-mongo-acaRemote: azure-samples-stagingBranch: pr/799You can initialize this project with: azd init -t Azure-Samples/todo-nodejs-mongo-aca -b pr/799View Changes | Compare Changes Project: todo-nodejs-mongo-swa-funcRemote: azure-samples-stagingBranch: pr/799You can initialize this project with: azd init -t Azure-Samples/todo-nodejs-mongo-swa-func -b pr/799View Changes | Compare Changes Project: todo-nodejs-mongoRemote: azure-samples-stagingBranch: pr/799You can initialize this project with: azd init -t Azure-Samples/todo-nodejs-mongo -b pr/799View Changes | Compare Changes Project: todo-nodejs-mongo-terraformRemote: azure-samples-stagingBranch: pr/799You can initialize this project with: azd init -t Azure-Samples/todo-nodejs-mongo-terraform -b pr/799View Changes | Compare Changes Project: todo-python-mongo-acaRemote: azure-samples-stagingBranch: pr/799You can initialize this project with: azd init -t Azure-Samples/todo-python-mongo-aca -b pr/799View Changes | Compare Changes Project: todo-python-mongo-swa-funcRemote: azure-samples-stagingBranch: pr/799You can initialize this project with: azd init -t Azure-Samples/todo-python-mongo-swa-func -b pr/799View Changes | Compare Changes Project: todo-python-mongoRemote: azure-samples-stagingBranch: pr/799You can initialize this project with: azd init -t Azure-Samples/todo-python-mongo -b pr/799View Changes | Compare Changes Project: todo-python-mongo-terraformRemote: azure-samples-stagingBranch: pr/799You can initialize this project with: azd init -t Azure-Samples/todo-python-mongo-terraform -b pr/799View Changes | Compare Changes |
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsStandalone Binary
Container |
|
I will test today and let you know
regards
Rory
…On Wed, Oct 5, 2022 at 12:30 AM Wei Lim ***@***.***> wrote:
@jdubois <https://github.com/jdubois>, @brunoborges
<https://github.com/brunoborges>, @roryp <https://github.com/roryp> or
@bbenz <https://github.com/bbenz> if any one of you can help sanity check
the Java portion of changes to make sure nothing is amiss, that'd be
appreciated.
I'm still waiting to get a daily azd build published so that you can run
without having to compile a dev build locally. In the meantime, it requires
following contributing.md
<https://github.com/Azure/azure-dev/blob/main/cli/azd/CONTRIBUTING.md> to
build an azd locally..
—
Reply to this email directly, view it on GitHub
<#799 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABQXZAA242SDPPKSNAJ3DRTWBSV2FANCNFSM6AAAAAAQ33WMSU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
It seems the docker device runs out of space for the pr/551
I suggest creating the docker mount with double the storage space.
#4 extracting
sha256:466cf5840e59205c075ee1121d8cfb41cac9c8d64d1fade97d9fa35f34460169 3.9s
#4 ERROR: failed to register layer: Error processing tar file(exit status
1): write /usr/lib/jvm/msopenjdk-17/lib/modules: no space left on device
------
[build 1/9] FROM
***@***.***:be60f04045428da1c8604897aaf96d7a178e4f44d07cc2981e6c49dc0a3b24b5
:
------
failed to register layer: Error processing tar file(exit status 1): write
/usr/lib/jvm/msopenjdk-17/lib/modules: no space left on device
: exit status 1
Rorys-MacBook-Pro-3:todo-java-mongo-aca rorypreddy$
…On Wed, Oct 5, 2022 at 12:30 AM Wei Lim ***@***.***> wrote:
@jdubois <https://github.com/jdubois>, @brunoborges
<https://github.com/brunoborges>, @roryp <https://github.com/roryp> or
@bbenz <https://github.com/bbenz> if any one of you can help sanity check
the Java portion of changes to make sure nothing is amiss, that'd be
appreciated.
I'm still waiting to get a daily azd build published so that you can run
without having to compile a dev build locally. In the meantime, it requires
following contributing.md
<https://github.com/Azure/azure-dev/blob/main/cli/azd/CONTRIBUTING.md> to
build an azd locally..
—
Reply to this email directly, view it on GitHub
<#799 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABQXZAA242SDPPKSNAJ3DRTWBSV2FANCNFSM6AAAAAAQ33WMSU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Full failure log for pr/551 for container apps template:
Rorys-MacBook-Pro-3:todo-java-mongo-aca rorypreddy$ azd up
? Please enter a new environment name: todocosmosrpza
? Please select an Azure location to use: 41. (US) East US 2 (eastus2)
? Please select an Azure Subscription to use: 23. ca-ropreddy-demo-test
(296f38ef-7319-4a2b-ab48-2f38cb21966e)
Infrastructure provisioning plan completed successfully
Provisioning Azure resources can take some time.
You can view detailed progress in the Azure Portal:
https://portal.azure.com/#blade/HubsExtension/DeploymentDetailsBlade/overview/id/%2Fsubscriptions%2F296f38ef-7319-4a2b-ab48-2f38cb21966e%2Fproviders%2FMicrosoft.Resources%2Fdeployments%2Ftodocosmosrpza
Created Resource group: rg-todocosmosrpza
Created Key vault: kv-ozejguycfkida
Created Log Analytics workspace: log-ozejguycfkida
Created Application Insights: appi-ozejguycfkida
Created Portal dashboard: dash-ozejguycfkida
Created Container Apps Environment: cae-ozejguycfkida
Created Container App: ca-api-ozejguycfkida
Created Azure Cosmos DB: cosmos-ozejguycfkida
Created Container App: ca-web-ozejguycfkida
Azure resource provisioning completed successfully
Deploying service api...
Error: deploying service: packaging service api: building container: api at
.: building image: exit code: 1, stdout: , stderr: #1 [internal] load build
definition from Dockerfile
#1 sha256:fea6948161f7e274462dc7e31ba847b9d8313b1bb5da0f8bfff5969805da2c4c
#1 transferring dockerfile: 777B done
#1 DONE 0.0s
#2 [internal] load .dockerignore
#2 sha256:86779696bd4f4c763a30f89cd74f33ce2a9dce8c7318b574cc918e1807f0460d
#2 transferring context: 2B done
#2 DONE 0.0s
#3 [internal] load metadata for mcr.microsoft.com/openjdk/jdk:17-mariner
#3 sha256:12da1caa7cb1e091da203d54e325c61d9dc8868901554bda381e714294745d9a
#3 DONE 3.7s
#6 [internal] load build context
#6 sha256:7e1d23fea746ca32373c4dd81c12ef1f6db700441d7bd37e8a50243d2ec39958
#6 transferring context: 132.34kB 0.0s done
#6 DONE 0.0s
#4 [build 1/9] FROM
***@***.***:be60f04045428da1c8604897aaf96d7a178e4f44d07cc2981e6c49dc0a3b24b5
#4 sha256:8c1a155b388143167348573bd9e32802c92d6857d1230f43b95dc2583eb670b9
#4 resolve
***@***.***:be60f04045428da1c8604897aaf96d7a178e4f44d07cc2981e6c49dc0a3b24b5
done
#4 sha256:466cf5840e59205c075ee1121d8cfb41cac9c8d64d1fade97d9fa35f34460169
149.69MB / 149.69MB 112.3s done
#4 sha256:be60f04045428da1c8604897aaf96d7a178e4f44d07cc2981e6c49dc0a3b24b5
742B / 742B done
#4 sha256:1e4ab1988ec292ad33262cfc7922458c62c8ecc5f0b43e8a518bdad9688f9c7c
5.88kB / 5.88kB done
#4 sha256:7f0517a4d0b40accc745957dd7518369b86e064df31d5c75fd94af60b0dd5e4e
28.09MB / 28.09MB 15.2s done
#4 extracting
sha256:7f0517a4d0b40accc745957dd7518369b86e064df31d5c75fd94af60b0dd5e4e
1.1s done
#4 extracting
sha256:466cf5840e59205c075ee1121d8cfb41cac9c8d64d1fade97d9fa35f34460169 3.9s
#4 ERROR: failed to register layer: Error processing tar file(exit status
1): write /usr/lib/jvm/msopenjdk-17/lib/modules: no space left on device
------
[build 1/9] FROM
***@***.***:be60f04045428da1c8604897aaf96d7a178e4f44d07cc2981e6c49dc0a3b24b5
:
------
failed to register layer: Error processing tar file(exit status 1): write
/usr/lib/jvm/msopenjdk-17/lib/modules: no space left on device
: exit code: 1, stdout: , stderr: #1 [internal] load build definition from
Dockerfile
#1 sha256:fea6948161f7e274462dc7e31ba847b9d8313b1bb5da0f8bfff5969805da2c4c
#1 transferring dockerfile: 777B done
#1 DONE 0.0s
#2 [internal] load .dockerignore
#2 sha256:86779696bd4f4c763a30f89cd74f33ce2a9dce8c7318b574cc918e1807f0460d
#2 transferring context: 2B done
#2 DONE 0.0s
#3 [internal] load metadata for mcr.microsoft.com/openjdk/jdk:17-mariner
#3 sha256:12da1caa7cb1e091da203d54e325c61d9dc8868901554bda381e714294745d9a
#3 DONE 3.7s
#6 [internal] load build context
#6 sha256:7e1d23fea746ca32373c4dd81c12ef1f6db700441d7bd37e8a50243d2ec39958
#6 transferring context: 132.34kB 0.0s done
#6 DONE 0.0s
#4 [build 1/9] FROM
***@***.***:be60f04045428da1c8604897aaf96d7a178e4f44d07cc2981e6c49dc0a3b24b5
#4 sha256:8c1a155b388143167348573bd9e32802c92d6857d1230f43b95dc2583eb670b9
#4 resolve
***@***.***:be60f04045428da1c8604897aaf96d7a178e4f44d07cc2981e6c49dc0a3b24b5
done
#4 sha256:466cf5840e59205c075ee1121d8cfb41cac9c8d64d1fade97d9fa35f34460169
149.69MB / 149.69MB 112.3s done
#4 sha256:be60f04045428da1c8604897aaf96d7a178e4f44d07cc2981e6c49dc0a3b24b5
742B / 742B done
#4 sha256:1e4ab1988ec292ad33262cfc7922458c62c8ecc5f0b43e8a518bdad9688f9c7c
5.88kB / 5.88kB done
#4 sha256:7f0517a4d0b40accc745957dd7518369b86e064df31d5c75fd94af60b0dd5e4e
28.09MB / 28.09MB 15.2s done
#4 extracting
sha256:7f0517a4d0b40accc745957dd7518369b86e064df31d5c75fd94af60b0dd5e4e
1.1s done
#4 extracting
sha256:466cf5840e59205c075ee1121d8cfb41cac9c8d64d1fade97d9fa35f34460169 3.9s
#4 ERROR: failed to register layer: Error processing tar file(exit status
1): write /usr/lib/jvm/msopenjdk-17/lib/modules: no space left on device
------
[build 1/9] FROM
***@***.***:be60f04045428da1c8604897aaf96d7a178e4f44d07cc2981e6c49dc0a3b24b5
:
------
failed to register layer: Error processing tar file(exit status 1): write
/usr/lib/jvm/msopenjdk-17/lib/modules: no space left on device
: exit status 1
…On Wed, Oct 5, 2022 at 12:30 AM Wei Lim ***@***.***> wrote:
@jdubois <https://github.com/jdubois>, @brunoborges
<https://github.com/brunoborges>, @roryp <https://github.com/roryp> or
@bbenz <https://github.com/bbenz> if any one of you can help sanity check
the Java portion of changes to make sure nothing is amiss, that'd be
appreciated.
I'm still waiting to get a daily azd build published so that you can run
without having to compile a dev build locally. In the meantime, it requires
following contributing.md
<https://github.com/Azure/azure-dev/blob/main/cli/azd/CONTRIBUTING.md> to
build an azd locally..
—
Reply to this email directly, view it on GitHub
<#799 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABQXZAA242SDPPKSNAJ3DRTWBSV2FANCNFSM6AAAAAAQ33WMSU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Created Container Apps Environment: cae-ozejguycfkida
Error: deploying infrastructure: error deploying infrastructure: failed
deploying: failed running az deployment sub create:
Deployment Error Details:
ConflictError: A vault with the same name already exists in deleted state.
You need to either recover or purge existing key vault. Follow this link
https://go.microsoft.com/fwlink/?linkid=2149745 for more information on
soft delete.next issue: I tried to manually delete the resource group after
the failure and cant recreate the app as the keyvault needs to be manually
deleted:
…On Wed, Oct 5, 2022 at 12:30 AM Wei Lim ***@***.***> wrote:
@jdubois <https://github.com/jdubois>, @brunoborges
<https://github.com/brunoborges>, @roryp <https://github.com/roryp> or
@bbenz <https://github.com/bbenz> if any one of you can help sanity check
the Java portion of changes to make sure nothing is amiss, that'd be
appreciated.
I'm still waiting to get a daily azd build published so that you can run
without having to compile a dev build locally. In the meantime, it requires
following contributing.md
<https://github.com/Azure/azure-dev/blob/main/cli/azd/CONTRIBUTING.md> to
build an azd locally..
—
Reply to this email directly, view it on GitHub
<#799 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABQXZAA242SDPPKSNAJ3DRTWBSV2FANCNFSM6AAAAAAQ33WMSU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
after fixeding the 2 errors (adding more docker storage locally on my
laptop, and manually purging the keyvault) I have confirmed the app is now
working perfectly [image: image.png]
…On Wed, Oct 5, 2022 at 12:30 AM Wei Lim ***@***.***> wrote:
@jdubois <https://github.com/jdubois>, @brunoborges
<https://github.com/brunoborges>, @roryp <https://github.com/roryp> or
@bbenz <https://github.com/bbenz> if any one of you can help sanity check
the Java portion of changes to make sure nothing is amiss, that'd be
appreciated.
I'm still waiting to get a daily azd build published so that you can run
without having to compile a dev build locally. In the meantime, it requires
following contributing.md
<https://github.com/Azure/azure-dev/blob/main/cli/azd/CONTRIBUTING.md> to
build an azd locally..
—
Reply to this email directly, view it on GitHub
<#799 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABQXZAA242SDPPKSNAJ3DRTWBSV2FANCNFSM6AAAAAAQ33WMSU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
I just tested the github actions via a private repo and it works great! When will you publish the live template @weikanglim ? |
Add SimpleTodo templates for Java AppService and Container Apps Co-authored-by: @jdubois and @brunoborges
| Optional<TodoItem> todoItem = getTodoItem(listId, itemId); | ||
| if (todoItem.isPresent()) { | ||
| todoItemRepository.deleteById(itemId); | ||
| return ResponseEntity.status(HttpStatus.NO_CONTENT).build(); |
There was a problem hiding this comment.
ResponseEntity.noContent().build();
| Optional<TodoItem> todoItem = getTodoItem(listId, itemId); | ||
| if (todoItem.isPresent()) { | ||
| todoItemRepository.deleteById(itemId); | ||
| return ResponseEntity.status(HttpStatus.NO_CONTENT).build(); |
There was a problem hiding this comment.
ResponseEntity.noContent().build()
| this.name = name; | ||
| } | ||
|
|
||
| public TodoItem description(String description) { |
There was a problem hiding this comment.
we don't usually use both fluent/chainable accessors and getter/setters at the same time.
We suggest to keep only getters/setters because they are more generally used and supported by other tools such as jackson
| Objects.equals(this.description, todoItem.description) && | ||
| Objects.equals(this.state, todoItem.state) && | ||
| Objects.equals(this.dueDate, todoItem.dueDate) && | ||
| Objects.equals(this.completedDate, todoItem.completedDate); |
There was a problem hiding this comment.
It is the same Todo item, no matter it is in the initial state, in progress, or done.
that is, I don't think properties like state should be used to implement equals()/hashCode() methods, because state is naturally to be updated for a todo item.
I would suggest keep only listId and id.
| TodoList todoList = (TodoList) o; | ||
| return Objects.equals(this.id, todoList.id) && | ||
| Objects.equals(this.name, todoList.name) && | ||
| Objects.equals(this.description, todoList.description); |
| @Override | ||
| public ResponseEntity<List<TodoItem>> getItemsByListId(String listId, BigDecimal top, BigDecimal skip) { | ||
| if (top == null) { | ||
| top = new BigDecimal(20); |
There was a problem hiding this comment.
you can use @RequestParam(defaultValue=20) to simplify code here and other.
https://www.baeldung.com/spring-request-param#a-default-value-for-the-request-parameter
| public interface TodoItemRepository extends MongoRepository<TodoItem, String> { | ||
|
|
||
| @Query("{ 'listId' : ?0 }") | ||
| List<TodoItem> findTodoItemsByTodoList(String listId); |
There was a problem hiding this comment.
I guess you don't need to add this @Query if this method is renamed as
findTodoItemsBy**ListId**(String listId);, because TodoItem itself has a property named listId, spring-data would generate the query by itself.
| public ResponseEntity<TodoList> updateListById(String listId, TodoList todoList) { | ||
| return todoListRepository | ||
| .findById(listId) | ||
| .map(t -> ResponseEntity.ok(todoListRepository.save(t))) |
There was a problem hiding this comment.
should be
ResponseEntity.ok(todoListRepository.save(**todoList**)))
otherwise the todolist won't be updated.
There was a problem hiding this comment.
we should also make sure the listId is set into todoList before saving. a new Todo List would be created after saving if its id is not set.
| return todoListRepository | ||
| .findById(listId) | ||
| .map(t -> ResponseEntity.ok(todoListRepository.save(t))) | ||
| .orElseGet(() -> ResponseEntity.badRequest().build()); |
There was a problem hiding this comment.
not sure, but I think it should return notFound() here.
| @JsonCreator | ||
| public static TodoState fromValue(String value) { | ||
| for (TodoState b : TodoState.values()) { | ||
| if (b.value.equals(value)) { |
There was a problem hiding this comment.
I would recommend to use equalsIgnoreCase() to be more robust.
| } | ||
|
|
||
| @Override | ||
| public ResponseEntity<Void> updateItemsStateByListId(String listId, TodoState state, List<String> requestBody) { |
There was a problem hiding this comment.
remove param requestBody if not used.
|
@wangmingliang-ms I agree with your PR review but be careful: many of the issues come because we inherit com.microsoft.azure.simpletodo.api.ListsApi which clearly isn't perfect, but is automatically generated using https://openapi-generator.tech (and to be more specific: their Spring code generator). As a result, unfortunately some of the methods cannot be changed, as they are inherited from this base class: sometimes that's because our Swagger contract isn't good (I found 3 errors in the contract that way), sometimes it's because OpenAPI doesn't generate perfect code (but we can contribute back to the project and fix it). |
|
@wangmingliang-ms - Would you like to do a PR for these changes? Thanks! Jon |
sure, I am happy to do that. |
|
Thanks you! |
Hi, @jongio here is the PR: #1122, it also fixes some other issues in addition to the above. |
8 participants
This PR adds SimpleTodo templates for Java AppService and Container Apps. Fixes #415. Note: Java support was added in
azdwith #773.Manually tested:
azd up, monitor, downcommandsThanks @jdubois and @brunoborges for the initial SimpleTodo API implementation.